Skip to content

Conversation

@marnovandermaas
Copy link
Contributor

@marnovandermaas marnovandermaas commented Nov 5, 2025

Instead of hardcoding 33 and 2 all over the code base as the most significant and least significant bit of the PMP address, this change creates appropriate parameters in the Ibex package that can be re-used in the rest of the code base.

This commit also replaces some hard-coded 16 values with PMP_MAX_REGIONS which was already a parameter in the Ibex package.

Finally it makes a slight clarifying change to avoid mixing pmp_mseccfg and pmp_mseccfg_q in the same assignment even though they refer to the same value.

@marnovandermaas
Copy link
Contributor Author

Keeping this PR as draft until I confirm the PMP pass rate is unchanged.

@marnovandermaas
Copy link
Contributor Author

marnovandermaas commented Nov 19, 2025

I've now run Ibex DV on this:

Test name Pass rate Diff from base
riscv_pmp_basic_test 50/50 =0
riscv_pmp_disable_all_regions_test 50/50 =0
riscv_pmp_out_of_bounds_test 50/50 +1
riscv_pmp_full_random_test 50/50
riscv_pmp_region_exec_test 20/20 =0
riscv_epmp_mml_test 20/20 =0
riscv_epmp_mml_execute_only_test 20/20 =0
riscv_epmp_mml_read_only_test 20/20 =0
riscv_epmp_mmwp_test 20/20 =0
riscv_epmp_rlb_test 20/20 =0

So now marking this as ready for review.

@marnovandermaas marnovandermaas marked this pull request as ready for review November 19, 2025 14:38
Copy link
Contributor

@SamuelRiedel SamuelRiedel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Marno. I have two small nits, but otherwise it looks good to me. I also ran a quick synthesis on your changes and it looks good.

Instead of hardcoding 33 and 2 all over the code base as the most
significant and least significant bit of the PMP address, this change
creates appropriate parameters in the Ibex package that can be re-used
in the rest of the code base.

This commit also replaces some hard-coded 16 values with PMP_MAX_REGIONS
which was already a parameter in the Ibex package.

I thought of adding a parameter for the least significant bit of the PMP
address while analysing a question posed by Shutong Jin during the
HACK@CHES'25 event.
Functionally pmp_mseccfg and pmp_mseccfg_q are the same when PMP is
enabled. However, the RTL is a bit more clear if both the MML and RLB
register are referenced through the pmp_mseccfg_q register name.

I thought of this clarification while analysing a question posed by
Shutong Jin during the HACK@CHES'25 event.
@marnovandermaas marnovandermaas added this pull request to the merge queue Nov 21, 2025
Merged via the queue into lowRISC:master with commit 0a13971 Nov 21, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants